-
Notifications
You must be signed in to change notification settings - Fork 208
chore: Add project_settings_api acceptance tests #3751
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore: Add project_settings_api acceptance tests #3751
Conversation
group_id = %[1]q | ||
is_collect_database_specifics_statistics_enabled = %[2]t | ||
is_data_explorer_enabled = %[2]t | ||
is_data_explorer_gen_ai_features_enabled = %[2]t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: Noticed this is the only attribute that is optional-only, does the API behave differently or we simply missed this additional config?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, adding config for this one.
{ | ||
Config: config(projectID, false), | ||
Check: check(projectID, false), | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Could be could to add a step where some or all attributes are not defined, given they are O+C values in state would not change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👌 Added an empty config step.
is_data_explorer_gen_ai_features_enabled = %[2]t | ||
is_data_explorer_gen_ai_sample_document_passing_enabled = %[2]t | ||
is_extended_storage_sizes_enabled = %[2]t | ||
is_performance_advisor_enabled = %[2]t |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: passing the same value to attributes could hide some assignment errors (e.g. we put in is_performance_advisor_enabled the value of is_extended_storage_sizes_enabled)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered this but decided against testing each value individually as of now.
How do we approach this typically? For the handwritten project resource we are not testing individual assignments so probably not worth doing it here either but open to suggestions.
|
||
check := resource.ComposeAggregateTestCheckFunc( | ||
checkExists(resourceName), | ||
resource.TestCheckResourceAttr(resourceName, "group_id", projectID), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
consider using some of the helper functions we have to group test checks, e.g. AddAttrChecks and similar
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done 6d0aae5
Description
Add acceptance tests for project_settings_api
Link to any related issue(s): CLOUDP-347907
Type of change:
Required Checklist:
Further comments